Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[WIP] abi2 prototype #468

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[WIP] abi2 prototype #468

wants to merge 3 commits into from

Conversation

a1batross
Copy link
Member

This is just a prototype. Now made in more thoughtful process, and proper class wrapper that helps to avoid string_t<>int conversions.

Though I'm really have mixed feelings on how it turned it anyway, especially with broken engine headers.

@a1batross
server: use string_t type where it was intended to be used, without b…
db835ff 
…roken engine headers

* It must be safest commit in this branch, as we only replace int by string_t,
  which is already int by default.
@a1batross
engine: start breaking engine headers
6255ffb 
* Entity Interface call pfnCreateNamedEntity actually accepts string_t
* viewmodel and weaponmodel in entvars_t are actually used as string_t
@a1batross
server: real ABI2 implementation
432ee6d 
* we define a new class called string_t that's only allowed
  to be converted from and to ptrdiff_t
* it specifically doesn't allow conversion to int on 64-bit platform
  only to ptrdiff_t. For now it just checks defined XASH_64BIT macro
  but check better be remade with some std::enable_if magic
* mark ABI2 binaries with _st64 prefix
@a1batross
Copy link
Member Author

I also think it must be pretty much possible to load both abi1 and abi2 binaries on engine side.

@a1batross
Copy link
Member Author

Stuff like pev->team = MAKE_STRING is absolutely stupid.

It's already defined in save restore as FIELD_INTEGER.

I think the string could be moved elsewhere, I don't see where it's used by the engine, and it's not really used by game dll as a string value that much. But it's more porting existing code compared to ABI1 anyway.

@FreeSlave
Copy link
Member

Could you provide more context on what it is?
Proper support for strings on 64-bit platforms?

@a1batross
Copy link
Member Author

a1batross commented Sep 5, 2024 •
edited
Loading

@FreeSlave yeah. Historically, string_t workaround was made in an attempt to load original GoldSource 64-bit binaries, like cs_amd64.so which still present in Counter-Strike distribution.

But it has few downsides:

  1. It tries to allocate the string base somewhere closer to the server library, which isn't reliable on almost all platforms.
  2. It makes MAKE_STRING optimization useless, because we can't store 64-bit pointer in a 32-bit data type, obviously.
  3. It forces server.dll to rely on engine's string pool, making ALLOC_STRING much more expensive, potentially being able to exhaust the string pool (though that never happened to me).
  4. ALLOC_STRING's deduplication mechanism potentially breaks the API, and it's enabled by default.

By just making string_t larger these problems become irrelevant. And we get few more.

@a1batross
Copy link
Member Author

And we get few more.

By making process of 64-bit porting a bit more complicated.

In short, first ABI was easier on HLSDK side but harder for engine. ABI2 makes it very easy for engine but harder for HLSDK and mostly because of fixing up for buggy string_t↔int conversions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@a1batross @FreeSlave